[clang][scan-deps] Add option to disable caching stat failures #144000
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-clang Author: Michael Spencer (Bigcheese) ChangesWhile the source code isn't supposed to change during a build, in some environments it does. This adds an option that disables caching of stat failures, meaning that source files can be added to the build during scanning. This adds a Full diff: https://github.com/llvm/llvm-project/pull/144000.diff 8 Files Affected:
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index a20a89a4c2b76..7a688082a55e1 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -350,7 +350,8 @@ class DependencyScanningWorkerFilesystem
DependencyScanningWorkerFilesystem(
DependencyScanningFilesystemSharedCache &SharedCache,
- IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
+ bool CacheNegativeStats = true);
llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override;
llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
@@ -491,6 +492,8 @@ class DependencyScanningWorkerFilesystem
/// using them for cache lookups.
llvm::ErrorOr<std::string> WorkingDirForCacheLookup;
+ bool CacheNegativeStats;
+
void updateWorkingDirForCacheLookup();
llvm::ErrorOr<StringRef>
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index 4e97c7bc9f36e..ceaf3c2279e7f 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -87,7 +87,8 @@ class DependencyScanningService {
ScanningOptimizations OptimizeArgs = ScanningOptimizations::Default,
bool EagerLoadModules = false, bool TraceVFS = false,
std::time_t BuildSessionTimestamp =
- llvm::sys::toTimeT(std::chrono::system_clock::now()));
+ llvm::sys::toTimeT(std::chrono::system_clock::now()),
+ bool CacheNegativeStats = true);
ScanningMode getMode() const { return Mode; }
@@ -99,6 +100,8 @@ class DependencyScanningService {
bool shouldTraceVFS() const { return TraceVFS; }
+ bool shouldCacheNegativeStats() const { return CacheNegativeStats; }
+
DependencyScanningFilesystemSharedCache &getSharedCache() {
return SharedCache;
}
@@ -116,6 +119,7 @@ class DependencyScanningService {
const bool EagerLoadModules;
/// Whether to trace VFS accesses.
const bool TraceVFS;
+ const bool CacheNegativeStats;
/// The global file system cache.
DependencyScanningFilesystemSharedCache SharedCache;
/// The global module cache entries.
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 140833050f4e9..e28c4362a0f2f 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -233,11 +233,12 @@ bool DependencyScanningWorkerFilesystem::shouldBypass(StringRef Path) const {
DependencyScanningWorkerFilesystem::DependencyScanningWorkerFilesystem(
DependencyScanningFilesystemSharedCache &SharedCache,
- IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, bool CacheNegativeStats)
: llvm::RTTIExtends<DependencyScanningWorkerFilesystem,
llvm::vfs::ProxyFileSystem>(std::move(FS)),
SharedCache(SharedCache),
- WorkingDirForCacheLookup(llvm::errc::invalid_argument) {
+ WorkingDirForCacheLookup(llvm::errc::invalid_argument),
+ CacheNegativeStats(CacheNegativeStats) {
updateWorkingDirForCacheLookup();
}
@@ -267,6 +268,8 @@ DependencyScanningWorkerFilesystem::computeAndStoreResult(
llvm::ErrorOr<llvm::vfs::Status> Stat =
getUnderlyingFS().status(OriginalFilename);
if (!Stat) {
+ if (!CacheNegativeStats)
+ return Stat.getError();
const auto &Entry =
getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError());
return insertLocalEntryForFilename(FilenameForLookup, Entry);
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
index 7f40c99f07287..c2f3cdbb02e37 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
@@ -15,7 +15,8 @@ using namespace dependencies;
DependencyScanningService::DependencyScanningService(
ScanningMode Mode, ScanningOutputFormat Format,
ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool TraceVFS,
- std::time_t BuildSessionTimestamp)
+ std::time_t BuildSessionTimestamp, bool CacheNegativeStats)
: Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs),
EagerLoadModules(EagerLoadModules), TraceVFS(TraceVFS),
+ CacheNegativeStats(CacheNegativeStats),
BuildSessionTimestamp(BuildSessionTimestamp) {}
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 9bd85479d9810..a54bcfdb509a0 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -611,8 +611,8 @@ DependencyScanningWorker::DependencyScanningWorker(
switch (Service.getMode()) {
case ScanningMode::DependencyDirectivesScan:
- DepFS =
- new DependencyScanningWorkerFilesystem(Service.getSharedCache(), FS);
+ DepFS = new DependencyScanningWorkerFilesystem(
+ Service.getSharedCache(), FS, Service.shouldCacheNegativeStats());
BaseFS = DepFS;
break;
case ScanningMode::CanonicalPreprocessing:
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 921ba7aadd67d..bb42f2f43aee6 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -85,6 +85,7 @@ static ScanningOutputFormat Format = ScanningOutputFormat::Make;
static ScanningOptimizations OptimizeArgs;
static std::string ModuleFilesDir;
static bool EagerLoadModules;
+static bool CacheNegativeStats = true;
static unsigned NumThreads = 0;
static std::string CompilationDB;
static std::optional<std::string> ModuleName;
@@ -191,6 +192,8 @@ static void ParseArgs(int argc, char **argv) {
EagerLoadModules = Args.hasArg(OPT_eager_load_pcm);
+ CacheNegativeStats = !Args.hasArg(OPT_no_cache_negative_stats);
+
if (const llvm::opt::Arg *A = Args.getLastArg(OPT_j)) {
StringRef S{A->getValue()};
if (!llvm::to_integer(S, NumThreads, 0)) {
@@ -1081,7 +1084,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
};
DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
- EagerLoadModules, /*TraceVFS=*/Verbose);
+ EagerLoadModules, /*TraceVFS=*/Verbose,
+ CacheNegativeStats);
llvm::Timer T;
T.startTimer();
diff --git a/clang/tools/clang-scan-deps/Opts.td b/clang/tools/clang-scan-deps/Opts.td
index 9cccbb3aaf0c8..582ae60851e1e 100644
--- a/clang/tools/clang-scan-deps/Opts.td
+++ b/clang/tools/clang-scan-deps/Opts.td
@@ -22,6 +22,7 @@ defm module_files_dir : Eq<"module-files-dir",
def optimize_args_EQ : CommaJoined<["-", "--"], "optimize-args=">, HelpText<"Which command-line arguments of modules to optimize">;
def eager_load_pcm : F<"eager-load-pcm", "Load PCM files eagerly (instead of lazily on import)">;
+def no_cache_negative_stats : F<"no-cache-negative-stats", "Don't cache stat failures">;
def j : Arg<"j", "Number of worker threads to use (default: use all concurrent threads)">;
diff --git a/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
index 683d9070b1dcf..3535f1a04f2be 100644
--- a/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
+++ b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
@@ -384,3 +384,53 @@ TEST(DependencyScanner, ScanDepsWithDiagConsumer) {
EXPECT_TRUE(DiagConsumer.Finished);
}
}
+
+TEST(DependencyScanner, NoNegativeCache) {
+ StringRef CWD = "/root";
+
+ auto VFS = new llvm::vfs::InMemoryFileSystem();
+ VFS->setCurrentWorkingDirectory(CWD);
+ auto Sept = llvm::sys::path::get_separator();
+ std::string HeaderPath =
+ std::string(llvm::formatv("{0}root{0}header.h", Sept));
+ std::string Test0Path = std::string(llvm::formatv("{0}root{0}test0.cpp", Sept));
+ std::string Test1Path = std::string(llvm::formatv("{0}root{0}test1.cpp", Sept));
+
+ VFS->addFile(Test0Path, 0,
+ llvm::MemoryBuffer::getMemBuffer(
+ "#if __has_include(\"header.h\")\n#endif"));
+ VFS->addFile(Test1Path, 0,
+ llvm::MemoryBuffer::getMemBuffer("#include \"header.h\""));
+
+ DependencyScanningService Service(
+ ScanningMode::DependencyDirectivesScan, ScanningOutputFormat::Make,
+ ScanningOptimizations::All, false, false,
+ llvm::sys::toTimeT(std::chrono::system_clock::now()), false);
+ DependencyScanningTool ScanTool(Service, VFS);
+
+ std::vector<std::string> CommandLine0 = {"clang",
+ "-target",
+ "x86_64-apple-macosx10.7",
+ "-c",
+ "test0.cpp",
+ "-o"
+ "test0.cpp.o"};
+ std::vector<std::string> CommandLine1 = {"clang",
+ "-target",
+ "x86_64-apple-macosx10.7",
+ "-c",
+ "test1.cpp",
+ "-o"
+ "test1.cpp.o"};
+
+ std::string Result;
+ ASSERT_THAT_ERROR(
+ ScanTool.getDependencyFile(CommandLine0, CWD).moveInto(Result),
+ llvm::Succeeded());
+
+ VFS->addFile(HeaderPath, 0, llvm::MemoryBuffer::getMemBuffer(""));
+
+ ASSERT_THAT_ERROR(
+ ScanTool.getDependencyFile(CommandLine1, CWD).moveInto(Result),
+ llvm::Succeeded());
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
jansvoboda11
approved these changes
Jun 13, 2025
Contributor
jansvoboda11
left a comment
jansvoboda11
left a comment
There was a problem hiding this comment.
LGTM. I think I would slightly prefer for DependencyScanningWorkerFilesystem to accept the entire service instead of its members (for the same reasons we do that in the worker, action, etc.)
benlangmuir
reviewed
Jun 13, 2025
Collaborator
There was a problem hiding this comment.
I think this is passing the flag to the BuildSessionTimestamp parameter.
Contributor
Author
There was a problem hiding this comment.
Oops, the downside to not being able to test it from here.
While the source code isn't supposed to change during a build, in some environments it does. This adds an option that disables caching of stat failures, meaning that source files can be added to the build during scanning. This adds a `-no-cache-negative-stats` option to clang-scan-deps to enable this behavior. There are no tests for clang-scan-deps as there's no reliable way to do so from it. A unit test has been added that modifies the filesystem between scans to test it.
Bigcheese
force-pushed
the
dev/no-negative-stat-cache
branch
from
4eef87a to
d07ccf2
Compare
Contributor
Author
I changed this, although now the tests need a service. I don't think that's a big deal as real use always has a service. It also required moving stuff out of the header as the service includes the vfs, so the service can't be a complete type in the vfs header. |
jansvoboda11
approved these changes
Jun 20, 2025
Member
|
Hi, I think this PR broke the clang unit tests. Can you take a look? |
qinkunbao
added a commit
that referenced
this pull request
Jun 24, 2025
…s" (#145528) Reverts #144000 First buildbot failure: https://lab.llvm.org/buildbot/#/builders/164/builds/11064
Member
|
Hi, I've reverted this PR according to https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jun 24, 2025
…tat failures" (#145528) Reverts llvm/llvm-project#144000 First buildbot failure: https://lab.llvm.org/buildbot/#/builders/164/builds/11064
DrSergei
pushed a commit
to DrSergei/llvm-project
that referenced
this pull request
Jun 24, 2025
…s" (llvm#145528) Reverts llvm#144000 First buildbot failure: https://lab.llvm.org/buildbot/#/builders/164/builds/11064
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
…s" (llvm#145528) Reverts llvm#144000 First buildbot failure: https://lab.llvm.org/buildbot/#/builders/164/builds/11064
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While the source code isn't supposed to change during a build, in some environments it does. This adds an option that disables caching of stat failures, meaning that source files can be added to the build during scanning.
This adds a
-no-cache-negative-statsoption to clang-scan-deps to enable this behavior. There are no tests for clang-scan-deps as there's no reliable way to do so from it. A unit test has been added that modifies the filesystem between scans to test it.